-
Notifications
You must be signed in to change notification settings - Fork 279
Jon/fix/edgespend #5900
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Jon/fix/edgespend #5900
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9bcf3e85c1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
9791d28 to
380ea4c
Compare
Jon-edge
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intentionally not bothering to migrate account format since we aren't in prod and any owned gift cards are currently fake.
swansontec
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only one React.FC to fix. The logs are fine if intentional (I think they are).
| } | ||
| throw new Error(`HTTP error! status: ${response.status} body: ${text}`) | ||
| } | ||
| debugLog('phaze', `Response: ${response.status} ${response.statusText}`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leftover, or intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intentional, yes, visibility is configured by env.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intentional, yes, visibility is configured by env.json
| headers: makeHeaders() | ||
| }) | ||
| const text = await response.text() | ||
| debugLog( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leftover, or intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intentional, yes, visibility is configured by env.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intentional, yes, visibility is configured by env.json
| } | ||
|
|
||
| export const CountdownTile = (props: Props) => { | ||
| export const CountdownTile = (props: Props): React.ReactElement | null => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
React.FC here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New eslint rule: #5902
Store Phaze identities as separate identifier-named files instead of one identifier-keyed file to address the issue of mutliple devices initializing the same Edge account with Phaze simultaneously stomping synced writes.
Route directly to Market scene if the Edge account doesn't have a Phaze account yet
- Fix cleaners to allow no-chainId-bearing currencies - Also update caip-19 documentation URL (changed since original impl)
Add forward-compatible workaround for Phaze API returning Polygon's native token precompile address (0x...1010) as an ERC20 instead of the correct slip44:966 format. This treats the precompile as native MATIC. If/when Phaze fixes it on their end, this workaround just becomes dead code.
380ea4c to
81721e9
Compare
- Fix parsing of expiry time - Show a clean internationalized countdown in `CountdownTile`
81721e9 to
c2e4ac8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
- Fully support non-USD fiats - Simplify and making it look neater by using whole numbers for conversions. - Global minimums from Phaze that are >$5 remain intact, e.g. XMR.
f8069f4 to
12cb165
Compare
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneRequirements
If you have made any visual changes to the GUI. Make sure you have:
Note
Improves the gift card experience and supporting utilities.
ErrorCard; fixquoteExpiryparsing and handle quote expiry viasend2onExpired; clean header title withcleanBrandName.giftCardListif a stored Phaze identity exists, otherwise togiftCardMarket;HomeTileCardonPresssupports async.onExpiredtoSendScene2;CountdownTileuses localizedformatCountdown; new i18n strings for quote expiry/product unavailable/countdown.phazeApiadds FX rate endpoints, response logging, and brand minimum filtering; provider pre-fetches FX rates, exposesgetCachedFxRates, and stores identities per-item; addhasStoredPhazeIdentityhelper.Written by Cursor Bugbot for commit 12cb165. This will update automatically on new commits. Configure here.